Skip to content

ZA | May-2025 | Fikiswa Matoti | Form controls #680

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 32 commits into
base: main
Choose a base branch
from

Conversation

FiksMat
Copy link

@FiksMat FiksMat commented Jun 2, 2025

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with REGION | COHORT_NAME | FIRST_NAME LAST_NAME | PROJ_NAME
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

This pull request submits the Form Controls assignment in which I created a form to collect customer data such as personal details, preferred t-shirt color and size.

Questions

Ask any questions you have for your reviewer.

FiksMat added 29 commits May 31, 2025 17:05
Copy link

netlify bot commented Jun 2, 2025

Deploy Preview for cyf-onboarding-module ready!

Name Link
🔨 Latest commit 1e8c258
🔍 Latest deploy log https://app.netlify.com/projects/cyf-onboarding-module/deploys/683ec9ed0d8c720008ceb2e8
😎 Deploy Preview https://deploy-preview-680--cyf-onboarding-module.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
2 paths audited
Performance: 100 (no change from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 86 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

@FiksMat FiksMat added the Needs Review Participant to add when requesting review label Jun 2, 2025
Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code is free of error and implementation is quite solid.

  1. In the PR description, can you also provide a brief description (under the "Changelist" section) summarizing the purpose of the PR and the changes you’ve made?
    image

  2. A PR branch should include only the changes relevant to the specific task or feature it addresses. This branch is created for the Form-Control exercise. As such, it should not include any modified files in the "Wireframe" folder.
    Could you revert the changes made in the "Wireframe" folder?
    One way to revert the changes is to replace the "Wireframe" folder in this branch by the "Wireframe" folder from the main branch.

<fieldset>
<legend><strong>Customer Details</strong></legend><br />
<div>
<label for="firstName" style="margin-right: 10px">First Name:</label>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec does say "No CSS". You form may not look as nice but can you remove the CSS code?

Comment on lines 32 to 43
<label for="red"><input type="radio" name="red-grey-black" id="red" value="Red" required />Red</label>
<label for="grey"><input type="radio" name="red-grey-black" id="grey" value="Grey" required />Grey</label>
<label for="black"><input type="radio" name="red-grey-black" id="black" value="Black" required />Black</label>
</fieldset><br />
<fieldset>
<legend><strong>Select your t-shirt size</strong>S</legend>
<label><input type="radio" name="xs-s-m-l-xl-xxl" id="xs" value="XS" required />XS</label>
<label><input type="radio" name="xs-s-m-l-xl-xxl" id="s" value="S" required />S</label>
<label><input type="radio" name="xs-s-m-l-xl-xxl" id="m" value="M" required />M</label>
<label><input type="radio" name="xs-s-m-l-xl-xxl" id="l" value="L" required />L</label>
<label><input type="radio" name="xs-s-m-l-xl-xxl" id="xl" value="XL" required />XL</label>
<label><input type="radio" name="xs-s-m-l-xl-xxl" id="xxl" value="XXL" required />XXL</label>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unusual name for name attributes. I don't think programmer working on this form will appreciate those names.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review and removed Needs Review Participant to add when requesting review labels Jun 3, 2025
@FiksMat
Copy link
Author

FiksMat commented Jun 3, 2025

Good morning CJ. Thank you for the review. I have updated my code accordingly.

Regarding the commit messages from my previous Wireframe PR appearing in the Form Controls PR, I followed the suggested steps, yet they still persist. Despite deleting the old Wireframe PR from GitHub, removing the Module-Onboarding folder from my local machine, and re-cloning it, the issue remains unresolved.

@cjyuan
Copy link
Contributor

cjyuan commented Jun 3, 2025

I was referring to the "Wireframe" folder (directory) in my message, nothing to do with the Wireframe PR or Wireframe branch.

  1. Download the main branch as a ZIP file from https://github.com/FiksMat/Module-Onboarding/tree/main
    - No need to unzip it
  2. Delete "Wireframe" folder from this branch
    - You need to make sure you have switched to form-controls branch while you do this.
  3. Copy "Wireframe" folder from the downloaded ZIP file
  4. Paste the folder to this branch
    - Actions 2-4 essentially replace the "Wireframe" folder
    - After you replace the folder, in VScode the Git extension should show 5 changes.
  5. Commit the changes
  6. Push commit to Github.

@FiksMat
Copy link
Author

FiksMat commented Jun 3, 2025

I have replaced the Wireframe folder.

@cjyuan
Copy link
Contributor

cjyuan commented Jun 3, 2025

Good job in fixing the branch.

You forgot to replace label. Too late now. Better luck in future PRs.

@cjyuan cjyuan added Complete Volunteer to add when work is complete and review comments have been addressed and removed Reviewed Volunteer to add when completing a review labels Jun 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complete Volunteer to add when work is complete and review comments have been addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants